Skip to content

No longer require crate-type = ["dylib"] on shader crates#477

Merged
Firestar99 merged 6 commits intomainfrom
cargo-rustc
Dec 5, 2025
Merged

No longer require crate-type = ["dylib"] on shader crates#477
Firestar99 merged 6 commits intomainfrom
cargo-rustc

Conversation

@Firestar99
Copy link
Member

@Firestar99 Firestar99 commented Dec 4, 2025 •
edited
Loading

With this PR, you no longer need to declare the crate-type in your shader crate.

All shader crates can now remove the previously required declaration in Cargo.toml below:

[lib]
crate-type = ["dylib"]

Changes

  • no longer requires crate-type to be set in your shader crate
  • switch spirv-builder from cargo build to cargo rustc
    • allows passing --crate-type=dylib
    • the difference between the commands is minor, rustc mostly just exposes additional rustc args for the target crate
  • the cargo <cmd> is configurable
    • future investigation: may allow calling other cargo commands, such as clippy or fmt

close #476

cargo-gpu

Rust-GPU/cargo-gpu#125

Needs to be updated after this PR is merged, and needs some fixups from other changes in master, so a simple cargo patch won't work.

This was referenced Dec 4, 2025
Closed
Merged
@Firestar99 Firestar99 enabled auto-merge December 4, 2025 13:57
LegNeato
LegNeato reviewed Dec 5, 2025
View reviewed changes
crates/spirv-builder/src/lib.rs
profile,
]);
if cargo_cmd_like_rustc {
cargo.args(["--crate-type", "dylib"]);
Copy link
Collaborator

@LegNeato LegNeato Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you include a comment for why we need dylib at all?

@Firestar99
Copy link
Member Author

Firestar99 commented Dec 5, 2025

Did another cleanup round, found some more crates that declare crate-type=["dylib"] that I missed, and added a quick comment on crate types:

rust-gpu/crates/spirv-builder/src/lib.rs

Lines 1026 to 1030 in d7681df

// About `crate-type`: We use it to determine whether the crate needs to be linked into shaders. For `rlib`,
// we're emitting regular rust libraries as is expected. For `dylib` or `cdylib`, we're linking all `rlib`s
// together, legalize them in many passes and emit a final `*.spv` file. Quirk: If you depend on a crate
// that has crate-type `dylib`, we also link it, and it will fail if it has no shaders, which may not be
// desired. (Gathered from reading source code and experimenting, @firestar99)

@Firestar99 Firestar99 mentioned this pull request Dec 5, 2025
Closed
@Firestar99
Copy link
Member Author

Firestar99 commented Dec 5, 2025

From cargo apk repository about using cargo rustc instead of cargo build:

The unfortunate downside of this, as shown by the last CI run is that we can no longer build multiple targets such as examples at once. Neither --examples nor explicitly specifying --example hello_world --example jni_audio --example looper is allowed, all result in error: crate types to rustc can only be passed to one target, consider filtering.

I don't think this affects us at all, since we're only ever building the --lib for shaders, never any examples or executables.

@Firestar99 Firestar99 added this pull request to the merge queue Dec 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 5, 2025
@LegNeato
Copy link
Collaborator

LegNeato commented Dec 5, 2025 •
edited
Loading

I'll let @eddyb review as I am not sure of ramifications.

@nazar-pc
Copy link
Contributor

nazar-pc commented Dec 5, 2025 •
edited
Loading

They already approved, but looks like the APIs used in the action to install Vulkan SDK on Windows is not using tokens, so it exceeds rate limits sometimes, which prevented merging: https://github.com/Rust-GPU/rust-gpu/actions/runs/19971068746/job/57275658889

@Firestar99 Firestar99 added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit e97524f Dec 5, 2025
13 checks passed
@Firestar99 Firestar99 deleted the cargo-rustc branch December 5, 2025 23:55
@nazar-pc nazar-pc mentioned this pull request Dec 12, 2025
Merged
Firestar99 added a commit to Rust-GPU/rust-gpu-template that referenced this pull request Dec 19, 2025
@Firestar99
update rust-gpu, remove dylib declaration
57e7902 
see Rust-GPU/rust-gpu#477
@Firestar99 Firestar99 mentioned this pull request Dec 19, 2025
Merged
Firestar99 added a commit to Rust-GPU/rust-gpu-template that referenced this pull request Dec 19, 2025
@Firestar99
update rust-gpu, remove dylib declaration
06ec042 
see Rust-GPU/rust-gpu#477
Firestar99 added a commit to Rust-GPU/rust-gpu-template that referenced this pull request Dec 19, 2025
@Firestar99
update rust-gpu, remove dylib declaration
8cfd029 
see Rust-GPU/rust-gpu#477
@Firestar99 Firestar99 mentioned this pull request Dec 22, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LegNeato LegNeato LegNeato left review comments

@eddyb eddyb eddyb approved these changes

@schell schell Awaiting requested review from schell schell is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove the need for crate-type = ["dylib"]

4 participants

@Firestar99 @LegNeato @nazar-pc @eddyb